r? @jdonszelmann

rustbot has assigned @jdonszelmann.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@bors try

☀️ Try build successful (CI)
Build commit: 6757d70 (6757d700f93f6d16c8b39cf79e96b019bd570e7d, parent: 12f35ad39ed3e39df4d953c46d4f6cc6c82adc96)

@craterbot check

👌 Experiment pr-155299 created and queued.
🤖 Automatically detected try build 6757d70
⚠️ Try build based on commit 5b074cf, but latest commit is 3bb6c15. Did you forget to make a new try build?
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

This looks nice Ralf, indeed let's nominate for lang after this completes

Note that this also forbids #[repr(transparent)] pub struct Phantomish<T>(PhantomData<T>) in addition to #[repr(C)] and #[repr(Rust)] 1ZSTs. My crate generativity exposes deliberately-PhantomData-like types with subtle safety invariants.

The custom phantom variance markers in core::marker use the internal #[rustc_pub_transparent] to avoid triggering this lint. There should be some option for custom user types before this becomes a hard error.

See also CAD97/generativity#13 where this was first reported as an issue with my crate. I didn't say anything at that point as I presumed it wouldn't be made into a hard error without some way to opt-in to being a trivial type for #[repr(transparent)]'s purposes.

Uh, that's interesting that you would assume that when the error very clearly states what our plans are and nothing in the tracking issue indicates other plans either... why did you wait years until the literal last moment before raising this concern upstream? You could have saved me a bunch of work by posting this any time in the last 3 years. :(

Note that this also forbids #[repr(transparent)] pub struct Phantomish<T>(PhantomData<T>) in addition to #[repr(C)] and #[repr(Rust)] 1ZSTs.

WDYM? This compiles without warning on stable:

use std::marker::PhantomData;

#[repr(transparent)]
pub struct Phantomish<T>(PhantomData<T>);

#[repr(transparent)]
pub struct Transparent<T>(u8, Phantomish<T>);

Edit: ah, I see, you need two crates…

FWIW I don't think it's necessary to block turning this into a hard error on some way to make 1ZST with private members. I just want to ensure that this knowingly effects #[repr(transparent)] of only PhantomData, and that providing a way of defining such is a known desire.

It's ultimately quite minor.

@CAD97 is there an issue tracking that? It'd be nice to have a good self-contained writeup for what you think is currently missing.

FWIW I'd also be fine with only making the repr(C) part of this a hard error, and letting the part that is motivated by semver concerns bake for longer. But there hasn't been any movement on that semver side nor even any suggestions for how the rules should be relaxed so that's not very actionable.

Let's see what crater says. Last time we tried this, we found only 2 regressions due to the semver rule; your crate did not show up. So either crater bugged out a bit (spurious failure in the baseline build?) or nothing covered by crater uses your crate with this pattern you mention.

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

Cc @obi1kenobi as this is about removing a currently-existing semver hazard 🎉

@CAD97 I think I finally understood your concern and made an issue: #155925.

Another breakage: the FCW currently triggers when one tries to use a PhantomData-like type defined using the ghost crate as an "extra" field in repr(tranparent).

That's exactly #155925 isn't it?

🚧 Experiment pr-155299 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

That's exactly #155925 isn't it?

Not just, repr(C) is used as well

That sounds like a bug in that crate then, repr(C) cannot be used for types that should be "ignored" by repr(transparent). That's the entire point.

IIUC, ghost is not using #[repr(C)] in the "compatible with C sense", but instead in the "predictable and stable layout" sense. They were actually trying to do the right thing according to our current documentation by not relying on unstable repr(Rust)

Yeah, I can see how they got there, but sadly it doesn't work... I filed an issue: dtolnay/ghost#41.

This is why I think we should hold the hard error until the repr(ordered_fields) RFC is merged. Maybe we want only the new repr(C) to actually correspond to C and be a hard error.